Memoize HermitCrab's sequential analysis cascade - #456
Conversation
5c42eaa to
8f5438a
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #456 +/- ##
==========================================
+ Coverage 73.30% 73.44% +0.14%
==========================================
Files 445 448 +3
Lines 37323 37509 +186
Branches 5120 5150 +30
==========================================
+ Hits 27360 27549 +189
+ Misses 8836 8826 -10
- Partials 1127 1134 +7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ddaspit
left a comment
There was a problem hiding this comment.
@ddaspit made 1 comment.
Reviewable status: 0 of 11 files reviewed, 1 unresolved discussion (waiting on johnml1135).
a discussion (no related file):
Before I do a full review, it would be great if you could rebase this on master. I suspect that some of the logic might need to be updated to work with the changes in PR #452.
8f5438a to
70bff72
Compare
|
Rebased this branch onto current Decisions made during the rebase:
Verification on final commit
|
70bff72 to
af80918
Compare
Adds AnalysisStateKey/AnalysisScope/Word.ReplayOnto and wires a memo table into both the mrule cascade and the affix-template battery, so repeated analysis-cascade states reached via different rule-unapplication orders are computed once and replayed rather than re-expanded. Off by default (the existing parallel cascade is unchanged); opt in via Morpher(maxDegreeOfParallelism: 1), which selects the new sequential+memo path. Ported from an archived prototype (parse-optimization-archive) with stronger verification: the acceptance gate is analysis-set equality (canonical morpheme-signature sets), not byte-identical objects, since a memo-replayed Word is not guaranteed field-for-field identical to a freshly-computed one. Verified via unit tests (key order-invariance, replay graft correctness, in-flight re-entry guard) plus corpus runs against three real grammars (Sena, Indonesian, Amharic) with zero analysis-set divergences. On a known-pathological word, sequential+memo measured 6.2x faster than the parallel default (isolated to ~6.3x attributable to the memo itself, not threading); aggregate corpus evidence and the typical-word tradeoff are in memoization.md, along with honestly-reported open gaps.
af80918 to
d9ccc1f
Compare
|
@ddaspit - it has been rebased and shouldn't have any issues. |
Summary
new Morpher(traceManager, language, maxDegreeOfParallelism: 1), which selects the new sequential+memo path.Wordisn't guaranteed field-for-field identical to a freshly-computed one, even when it represents the same analysis.See
memoization.md(added in this PR) for the full design, the key-completeness audit, and the verification methodology.Follow-up: #457 stacks a data-structure rearchitecture (array-backed, copy-on-write
Shape) on top of this branch and re-measures the same two heavy words — read that PR for whether it's worth the larger diff.Why off by default — the actual tradeoff
This is not a pure win, and the corpus evidence says so directly:
So flipping the library's default cascade mode trades typical-word latency for pathological-word latency. Whether that's the right tradeoff depends on a given corpus's word-difficulty distribution — a decision for a future PR with its own evidence, not this one.
Verification
AnalysisStateKeyorder-invariance/hash/equality,Word.ReplayOntograft correctness (including a test that specifically distinguishes "grafted the right subtree" from "grafted the wrong one" via distinct lexical entries), theInProgressin-flight re-entry guard, positive-replay-vs-unmemoized-result-set equivalence (including trail order), and an mrule/template equivalence battery against real analysis-rule content viaMorpherTests.memoization.mdrather than glossed over. (Stack RUSTIFY's array/COW rearchitecture on top of memoization #457 revisits H2 with the array-backed rearchitecture and gets a very different result.)Test plan
dotnet build Machine.slndotnet csharpier check .dotnet testacross HermitCrab, SIL.Machine, and Thot test projectsMemoCorpusVerification,[Explicit]) run manually against local Sena/Indonesian/Amharic grammars — 0 divergences on the words that completed within the timeout budgetThis change is